Skip to content

Conversation

danegsta
Copy link
Member

@danegsta danegsta commented Oct 16, 2025

Renames CustomCertificateAuthoritiesScope to CertificateTrustScope and adds new System and None values.

System behaves similar to the existing Override type, but automatically includes system trusted root certificates. None disables custom certificate trust for a resource.

Because Python doesn't support appending to default certificate trust (such as certifi certificates), this PR also makes the default for Python apps System.

@danegsta danegsta requested a review from davidfowl October 16, 2025 23:07
Copy link
Contributor

github-actions bot commented Oct 16, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12103

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12103"

@danegsta danegsta changed the title Use system certs as default for Python if Scope is Append Add new System and None scopes for certificate trust mode, use System by default for Python Oct 17, 2025
@danegsta danegsta marked this pull request as ready for review October 17, 2025 18:16
@Copilot Copilot AI review requested due to automatic review settings October 17, 2025 18:16
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces enhanced certificate trust configuration for Aspire resources by adding two new scope values (System and None) to CustomCertificateAuthoritiesScope. The System scope enables resources to trust both custom certificates and system root certificates, while None disables custom certificate trust entirely. Python applications now default to System scope since they cannot easily append to their default certificate stores (like certifi).

Key changes:

  • Added System and None enum values to CustomCertificateAuthoritiesScope with comprehensive documentation
  • Updated DCP executor logic to handle the new scopes, including system certificate retrieval for containers and executables
  • Changed Python apps to default to System scope and added SSL_CERT_FILE environment variable support

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Aspire.Hosting/ApplicationModel/CertificateAuthorityCollectionAnnotation.cs Added System and None enum values with XML documentation explaining their behavior
src/Aspire.Hosting/ResourceBuilderExtensions.cs Updated documentation to clarify default scope behavior
src/Aspire.Hosting/Dcp/DcpExecutor.cs Implemented handling for new scopes in both executable and container certificate trust logic
src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs Changed default scope to System for Python apps and added SSL_CERT_FILE support
src/Aspire.Hosting.NodeJs/NodeExtensions.cs Added SSL_CERT_FILE environment variable for Node.js apps using OpenSSL CA

@davidfowl
Copy link
Member

Code looks good, need to get sign off from a security POV.

@danegsta
Copy link
Member Author

Code looks good, need to get sign off from a security POV.

I’ll schedule a review (and let Steve know we specifically need his eyes on it since otherwise it just goes to the general on rotation reviewers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants